[9주차/여니] 워크북 제출합니다#58
Conversation
YoungJJun
left a comment
YoungJJun
left a comment
There was a problem hiding this comment.
9주차 피드백
-
필수, 선택 미션 모두 잘 구현해주셨습니다!
-
OAuth 제공자 확인하기 전에 하드코딩 추출하는 코드가 있는데 우선 워크북에서 카카오만 구현하기 때문에 이렇게 하신거겠죠??!
-
CustomOAuth2UserService - loadUser()
소셜 로그인 상황에서 멤버가 없는경우 생성해주는 로직이 있는데
@Transactional이 없습니다.예외 발생 시 롤백이 이루어지지 않을 수 있을 것 같아서 저장 로직이 포함되어 있으니
@Transactional추가해도 좋을 것 같아요. -
OAuthSuccessHandler
OAuthMember member = (OAuthMember) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
이미 파라미터로 전달된거 이용하는 방식 사용하면 될 것 같아요! 이게 다른 챌린저 코드에서도 똑같은 코드가 있었던 것 같은데 워크북에서 그렇게 알려준것 같습니다..!
authentication.getPrincipal()
-
AuthService - login()
@Transactional(readOnly = true)추가하면 어떨까요? -
JwtAuthFilter -
catch (Exception e)모든 예외를
UNAUTHORIZED로 처리 중인데 대부분의 문제가 JwtUtil을 사용하는 경우같고 이 경우 JwtUtil 내부에서 JwtException을 던지는 것 같아요.그러면 JwtAuthFilter 내부에서 발생할 수 있는 상황에는
500서버에러가 발생할 가능성이 있기는 한 것 같은데 이를 모두401로 던지는 것 같아요.500이 발생할 가능성이 커보이지는 않지만401로 통일하는건 위험할 수 있고 디버깅도 어려워질 것 같아요!
여니 수고하셨습니다! 시험 잘보세용~ 🍎
chazy-d
left a comment
chazy-d
left a comment
There was a problem hiding this comment.
여니 9주차 미션도 수고하셨습니다!! 시험 끝나고 다시 뵙겠습니다~~!! 😊
There was a problem hiding this comment.
OAuth user-info 조회는 Spring Security에 맡기고, 이후 Kakao attributes를 Member로 연결하는 흐름이 잘 보였습니다!!
저는 비슷한 코드를 domain.auth.oauth 쪽에 두었는데, 여니는 global.security.service에 두셔서
이 클래스를 Security 관할로 보신 것 같다는 인상을 받았습니다!
Member 조회/생성에 들어가는 OAuth 매핑 로직을 security 패키지에 두는 대신 auth 도메인에 두는 방식은 어떻게 생각하시는 궁금합니다!
There was a problem hiding this comment.
그린이 말씀해주신 내용에 공감합니다!! MemberRepository에 접근하고 실제로 객체를 생성하는 로직은 auth 패키지에 두는 게 훨씬 자연스러울 것 같습니다.
다만 Spring Security가 직접 호출하는 loadUser함수 자체는 프레임워크와 직접적으로 연관되어 있어서 도메인과는 분리하는 게 더 적절하다고 판단했습니다! 그래서 loadUser함수 자체가 OAuth인증의 오케스트레이터 역할을 할 수 있도록 각 provider에 맞는 DTO 생성과 member 조회 및 생성 로직을 호출하는 방식으로 리팩토링해봤습니다.
리팩토링하는 과정에서 그린의 코드도 많이 참고했습니다 ^^!! ㅎㅎ
그린 생각도 궁금합니다~ 혹시 다르게 보시는 부분 있으면 편하게 알려주세용 🤗🤗
| @Override | ||
| public OAuth2User loadUser( | ||
| OAuth2UserRequest userRequest | ||
| ) throws OAuth2AuthenticationException { | ||
| OAuth2User oAuth2User = super.loadUser(userRequest); | ||
|
|
||
| SocialType providerId; | ||
| String socialUid; | ||
| Map<String, Object> attributes = oAuth2User.getAttribute("kakao_account"); | ||
| Map<String, Object> profile = (Map<String, Object>) attributes.get("profile"); | ||
| try { | ||
| providerId = SocialType.valueOf(userRequest.getClientRegistration().getRegistrationId().toUpperCase()); | ||
| socialUid = String.valueOf((Long) oAuth2User.getAttribute("id")); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new AuthException(AuthErrorCode.NOT_SUPPORT_SOCIAL_PROVIDER); | ||
| } |
There was a problem hiding this comment.
워크북 범위에서는 바로 kakao_account를 꺼내는 여니의 방식도 충분히 간단하고 좋다고 생각합니다!!
조금더 넓게? 생각해보면 실제 서비스 기준에서는 provider별 파싱 메서드와 필수값 검증을 분리해두면 NPE 대신 OAuth 인증 실패로 처리하기 좋고, 나중에 다른 provider를 붙일 때도 변경 위치가 명확해질 것 같습니다!
조금 나누어 생각해보면, Kakao 하나만 사용하더라도 scope 설정이나 사용자 동의 여부에 따라 email/profile 값이 누락될 수 있어서 필수값 검증은 필요할 수 있다고 생각했습니다.
그리고 나중에 Google/GitHub/Naver처럼 provider가 늘어나면 user-info 응답 구조 자체가 각각의 provider 마다 달라질 수 있기 때문에, provider별 응답을 서비스의 공통 DTO나 Member 모델로 변환하는 계층을 분리하는 것도 의미가 있어 보입니다!
There was a problem hiding this comment.
워크북 내용만 따라가다보니 다른 provider를 어떻게 처리할지 고려를 못했네요!!
Kakao이외의 provider별 응답 파싱을 위해 OAuthAttributes 클래스를 따로 분리해서 역할을 나누고 응답 파싱 과정에서 필수 필드에 대한 검증 로직을 추가하는 방식으로 리팩토링해봤습니다!!
자세한 리뷰 감사합니다...😊
| try { | ||
| authentication = authenticationManager.authenticate( | ||
| new UsernamePasswordAuthenticationToken(reqDto.email(), reqDto.password()) | ||
| ); |
There was a problem hiding this comment.
일반 로그인에서 AuthenticationManager.authenticate()를 사용하셨군요!
저는 직접 findByEmail 후 passwordEncoder.matches()로 검증하는 흐름을 먼저 떠올렸는데, 이 방식은 Spring Security 인증 흐름을 그대로 따라 UserDetailsService, PasswordEncoder, provider 구조와 연결된다는 장점이 있는 것 같아요,
저도 이 방식으로 리팩토링해보면 좋을 것 같습니다!
3 participants
✅ 실습 체크리스트
✅ 컨벤션 체크리스트
📌 주안점